Fix Save As silently corrupting the file on BLE - #545
Open
dhalbert wants to merge 1 commit into
Open
Conversation
Save As wrote only the changed suffix into the new file, leaving everything
before that offset as whatever the freshly allocated cluster happened to
contain. Editing `print("Hello World!")` to add ` xxx` and saving as a new
file produced 26 bytes of which only the last 7 were correct:
2e 20 20 20 20 20 20 20 20 20 20 10 00 70 44 7d e7 5c e7 | 20 78 78 78 22 29 0a
`------------------ 19 bytes of junk -----------------' `--- " xxx")\n" ---'
19 is the length of `print("Hello World!`, the common prefix with the previous
contents. Save As with no edits at all was worse: the offset then equals the
whole document, so nothing was written and the file was junk end to end.
`unchanged` tracks the leading run of bytes known to match the device and is
used as the partial-write offset. saveFileContents() reset it when writing a
different file, but tested `path !== workflow.currentFilename`, and
saveFileAs() (workflow.js:467) assigns currentFilename *before* calling save.
The two therefore always matched by then and the reset never fired, so the
offset from the previously open file was applied to a brand-new one.
Rather than reorder saveFileAs(), track which file the offset describes.
`unchanged` is only meaningful for one path, so record it in `unchangedPath`
and reset when they disagree. That enforces the actual invariant and also
covers routes other than Save As that could leave the offset and the target
file out of step.
BLE only in practice: partialWrites is true just for BLEWorkflow, and the base
class defaults it to false, so the web and USB workflows always write in full.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Save As silently corrupts the file it writes when the BLE workflow is in use.
Reproduce
With a board connected over BLE, open
code.pycontainingprint("Hello World!"), addxxxbefore the closing paren, and Save As to a new name. The result is 26 bytes of which only the last 7 are correct:19 is the length of
print("Hello World!, the common prefix with the previous contents. Save As with no edits at all is worse: the offset then equals the whole document, nothing is written, and the file is junk end to end.Cause
unchangedtracks the leading run of bytes known to match the device and is used as the partial-write offset.saveFileContents()reset it when writing a different file, but testedpath !== workflow.currentFilename— andsaveFileAs()(js/workflows/workflow.js:467) assignscurrentFilenamebefore calling save. The two therefore always matched by then, the reset never fired, and the offset computed for the previously open file was applied to a brand-new one.Fix
Rather than reorder
saveFileAs(), track which file the offset describes.unchangedis only meaningful for one path, so record it inunchangedPathand reset when they disagree. That enforces the actual invariant and also covers routes other than Save As that could leave the offset and the target file out of step.BLE only in practice:
partialWritesis true just forBLEWorkflowand defaults to false in the base class, so the web and USB workflows always write in full.Testing
Feather nRF52840 Express, CircuitPython 10.3.0-alpha.4, Chrome 151 on macOS. Save As now produces byte-correct files, verified by reading the result off the CIRCUITPY drive. A plain Save after an edit still works and still uses a non-zero offset, so partial writes are not silently turned into full rewrites.